Skip to content

Fix some MSVC warnings #500

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed

Conversation

kdchambers
Copy link

Minor changes to reduce the number of compile warnings emitted. I'm using VMA in a unity build with MSVC and fairly strict compile settings in Debug mode. I saw it mentioned before that fixing compile warnings isn't considered important (Understandably), so it's fine to close the PR if the changes are considered against the preferences of the project.

Overview:

  • Mark unused variables with (void)var;
  • In VmaVector<T, AllocatorT>::resize remove ternary branch as it should not be possible for newCapacity to be 0. Apart from removing an unnecessary branch and clearing up intent, it removes a warning about potentially passing NULL as input to memcpy, which is undefined behavior.
  • Hoist compile time known macros out of runtime expressions

Notes:

  • There were 1 or 2 places where marking a variable as unused wasn't perfectly clean, for example if it's just used in a macro such as VMA_LEAK_LOG_FORMAT which may not may not expand to an expression. I figured it was still better to include them if warnings about unused variables aren't considered useful anyways, at least it reduces build noise.
  • Probably intentional, but VmaVector<T, AllocatorT>::resize will allocate a new buffer and copy over the contents even if the new capacity is less than the current. I don't know if there's an easy way to just shrink the allocation in such a case to avoid the overhead of allocating a new buffer and doing a memcpy. I didn't look into it for this PR.

The branch to reallocate the buffer should never be taken with
newCapacity = 0. So assert that and remove branch that leads the
compiler to believe that the following memcpy could be passed
a NULL as input
sawickiap added a commit to sawickiap/VulkanMemoryAllocator that referenced this pull request Jul 10, 2025
@sawickiap
Copy link
Contributor

sawickiap commented Jul 10, 2025

Thank you for these suggestions. I applied some of the fixes you proposed and some more, changing if to #ifdef, on my fork for now: https://github.com/sawickiap/VulkanMemoryAllocator

However, adding more lines of code with dummy statements just to silence warnings about unused variables is exactly what don't want to do, as described in the documentation, section Features not supported. I recommend disabling such warnings.

I don't think VmaVector<T, AllocatorT>::resize will reallocate and copy memory even when smaller size is requested. Please note that the function reallocates only when newCapacity != m_Capacity, new capacity is changed only when newCount > m_Capacity, and element count is never larger than the current capacity. Thus, the reallocation happens only when new requested element count is larger than the current capacity.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants